Skip to content

Update to edition 2024, MSRV 1.85#231

Merged
197g merged 2 commits into
image-rs:masterfrom
lilith:edition-2024-pr
Apr 11, 2026
Merged

Update to edition 2024, MSRV 1.85#231
197g merged 2 commits into
image-rs:masterfrom
lilith:edition-2024-pr

Conversation

@lilith
Copy link
Copy Markdown
Contributor

@lilith lilith commented Apr 10, 2026

Summary

  • Update Rust edition from 2021 to 2024
  • Bump MSRV from 1.62 to 1.85 (minimum for edition 2024)
  • Modernize CI actions (actions/checkout@v4, dtolnay/rust-toolchain)
  • Pin resolver = "2" to avoid edition 2024's v3 resolver default
  • Delete clippy.toml (clippy reads rust-version from Cargo.toml since 1.64)

Code changes required by edition 2024

  1. Doc-include macro: The extern "C" {} hack for embedding README doctests doesn't work in edition 2024 (unsafe extern is now required, which conflicts with #![forbid(unsafe_code)]). Replaced with the standard #[doc = include_str!("../README.md")] mod _readme {} pattern.

  2. Binding mode: Edition 2024 changes implicit reference binding in closures. |(_, &value)| becomes |&(_, &value)| in min_by_key (one occurrence in common.rs).

  3. is_none_or: Applied clippy's suggestion to replace map_or(true, ...) with is_none_or(...) (stabilized in 1.82, two occurrences).

All 39 tests pass. cargo msrv verify confirms 1.85.0 compatibility.

Discussion: consider going to 1.88 or 1.89

Since this is a large MSRV jump anyway (1.62 → 1.85), it may be worth going further. The image crate is already on MSRV 1.88, so any user pulling in gif through image already needs 1.88+.

Case for 1.88

  • [T]::as_chunks / as_rchunks — process pixel data in fixed-width chunks (3-byte RGB, 4-byte RGBA) without bounds checks or manual index math. Directly applicable to from_rgba_speed, from_rgb_speed, from_grayscale_with_alpha.
  • let chains (edition 2024 only) — if let Some(x) = a && let Some(y) = b replaces nested if let ladders in parser code.
  • image is already thereimage 0.25.10 requires 1.88, so the primary downstream consumer already demands it.

Case for 1.89

1.89 is where Rust's SIMD story came together across four releases:

  • 1.86: #[target_feature] functions can be safe — composing SIMD code no longer needs unsafe at every call site between matching feature contexts.
  • 1.87: Most std::arch intrinsics (arithmetic, shuffle, compare, bitwise) are now safe inside #[target_feature] functions. Only pointer-based ops (loadu/storeu) remain unsafe.
  • 1.88: as_chunks gives zero-cost fixed-size array views into slices — exactly what safe SIMD loads need.
  • 1.89: AVX-512 stabilized on stable Rust — 22 target features, ~857 intrinsics, plus SHA-512/SM3/SM4. Runtime detection (is_x86_feature_detected!("avx512f")) works on stable. The entire AVX-512 family was previously locked behind nightly-only feature gates.

Together, these mean a crate on MSRV 1.89 can use #![forbid(unsafe_code)] while still doing SIMD dispatch — no unsafe, no nightly, no feature gates. This matters for the broader image-rs ecosystem as codecs move toward safe SIMD. See archmage's MSRV rationale for the full breakdown.

This PR targets 1.85 as the conservative baseline. Happy to bump to 1.88 or 1.89 if preferred.

Test plan

  • cargo test --all-features — 39 tests pass
  • cargo clippy --all-features — clean (one pre-existing if_same_then_else warning)
  • cargo msrv verify — confirms 1.85.0
  • cargo fmt --check — clean
  • Tested top 20 reverse dependencies via cargo-copter — no regressions from the edition change itself

- Replace `extern "C" {}` doc-include hack with `#[doc = include_str!(...)]`
- Fix edition 2024 binding mode change in `min_by_key` closure
- Apply `is_none_or` (stabilized 1.82) replacing `map_or(true, ...)`
- Update CI to modern actions (checkout@v4, dtolnay/rust-toolchain)
- Sync clippy.toml MSRV

Verified: all 39 tests pass, `cargo msrv verify` confirms 1.85.0.
Comment thread Cargo.toml
Comment thread clippy.toml Outdated
- Add resolver = "2" to avoid edition 2024's v3 resolver default
- Delete clippy.toml; clippy reads rust-version from Cargo.toml since 1.64
@fintelia
Copy link
Copy Markdown
Contributor

Together, these mean a crate on MSRV 1.89 can use #![forbid(unsafe_code)] while still doing SIMD dispatch — no unsafe, no nightly, no feature gates.

This isn't true. You still need unsafe, it just gets hidden/abstracted. I was in the process of writing up a long response, but realized there's no point in arguing with what's probably an LLM hallucination.

Copy link
Copy Markdown
Member

@197g 197g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take this one with the version it really needs, i.e. as-is. With rust-version it makes little difference if we bump it now or later, so I'd rather do that on actual demand than on preemptive philosophistry.

Re: simd and llm debate: I've already looked at archmage before, it's at least is an interesting approach; fn(Token, ..) is a kind of newtype wrapper choice to annotate an original unsafe fn with; which is the direction I would have liked std's cpu-flag macro to have gone in from the start yet it has a huge design space. So it's good to explore one very clear direction here. There's a bit of duplication though and this together with almost everything happening inside proc-macros does, by-design, not aid the argument that off-loading unsafe into the crate makes the safety more reviewable. (Which is the important part since if we depended on cve-rs we could also forbid() but it'd be disastrous).

@197g 197g merged commit 6569cda into image-rs:master Apr 11, 2026
13 checks passed
@lilith
Copy link
Copy Markdown
Contributor Author

lilith commented Apr 11, 2026

I think there is an argument that we can trust is_*_feature_detected(x), and that you can make a provably sound abstraction on top of that which is simple enough to audit. That said, it does generate a trampoline, which does contain an unsafe block to call the inner function - the function itself is not unsafe annotated, though, so the only unsafety is the target_feature hop/switch. Rust itself computes the set math at compile time to determine if a call is safe or if you will be required to get a new capability token.

Archmage takes the generate-everything approach to ensure token types never get mismatched, and most of the complexity has been around how to group features into pragmatic supersets, name things, make such code testable, and ergonomics. Using a cpu instruction a cpu doesn't support is unsound, true, but in reality is quite like exit() rather than something exploitable, AFAIK. I haven’t had any such instructions slip through the cracks yet.

The dependency safe_unaligned_simd crate is more load-bearing, but trivial to audit in a different way - it's just checking slices-to-pointer logic. I don't own that crate, but I trust it.

Given archmage+forbid unsafe, I feel comfortable letting LLVMs prototype with intrinsics all day and explore every idea I can come up with.

Magetypes, on the other hand, takes an approach similar to fearless_simd (something I wish I had looked at earlier), and uses generics and associated types to make simd use 95% as fast as raw intrinsics. It is much harder to audit, so I parse the rust source code to build a db of which intrinsics rust considers safe per feature token, then use that for generating and validating the simd types. It's labeled experimental, but it's actually working better than I hoped. I haven’t run into any soundness holes with either crate yet.

@197g
Copy link
Copy Markdown
Member

197g commented Apr 12, 2026

The design problem of generating unsafe in a proc-macro is not the lack of unsafe on the surface, any encapsulation does that, it's the instability of encapsulation. Instead of auditing some piece of code as-is you need to certify the full range of possible outputs. That is after all the definition of soundness, correct under all inputs. The proc-macro as-implemented breaks the usual approach of having a module boundary with finite snippets that are self-contained encapsulations of the unsafe parts. You named the upside of this, the ability to have by-construction semantics, but the crate would need to be (more) rigorously structured for that. In the end I'll still trust a by-construction approach that leverages types and traits (the types themselves may be generated) more; I think. Not the least because this has a more reasonable avenue to be compatible with automated proof approaches (kani etc) where necessary.

There are more convincing ways to write a proc-macro; the derives of bytemuck and zerocopy do not just unsafe impl but generate a sufficient bound necessary for the impl to work. The actual proof is then (supposedly) done at compile time. I say supposedly since there are problems with those crates where having specific imports at the usage site can make them mistake types in the bound structure, they miss fields added by later proc-macros, etc. So really they are not sound. (Still useful but not sound).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants